-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrap cite with footer in saved markup for quote and pullquote blocks #9804
Wrap cite with footer in saved markup for quote and pullquote blocks #9804
Conversation
Test failures seem unrelated. |
Shouldn't any |
@joyously The I think it is assumed by the Quote block is that the only thing you are putting in the footer is the citation. I would not be opposed to removing the Additionally, most people are going to just put citations in the footer. If they want to get really fancy with what is in the footer, then I guess a Custom HTML block is the best answer for them. As it is, assuming that the only content in the footer is a citation seems fine to me, but if everyone else wants to remove the Whatever the case, this is more semantically correct than what is currently in |
It's not true that you don't need the attribute if you have a visible cite tag. The attribute is really for a URL. |
This feels like a nice iterative improvement over the existing behavior! A discussion on whether or not to move the citation out of the I think it'd be good to have another reviewer look at the console notice but semantically this makes sense to me and I like that it simplifies the CSS a smidge. Great stuff Zeb! |
Thanks for pointing that out. I was mistaken. I agree with @chrisvanpatten that the
That is intentional, because that is part of a definition of the deprecated form of the block, not the current form.
I agree with @chrisvanpatten that that might be a good thing to explore in another PR. However, as it is, the current method clearly associates the |
Due to recent changes, this PR needed a rebase due to merge conflicts, but so much had changed that it was easier to create a new PR entirely: #10465. |
Description
This PR changes the Quote and Pullquote blocks to wrap their
<cite>
s with<footer>
s. The reason for this is that<cite>
tags can be used within a quote but not be the citation of a quote, e.g. using a<cite>
element to refer to a book or ship. The use of a<footer>
improves the semantics to show that the contained<cite>
is the citation of the entire<blockquote>
, and makes it easier for themes to style inline<cite>
s differently from the one in the<footer>
.In the editor, the RichText field for the citation is a
<footer>
element, where previously it was a<div>
due to #8785 and before that it was a<cite>
.How has this been tested?
I have tested to make sure that old Quote and Pullquote blocks are automatically converted properly. Some errors appear in the JavaScript console the first time a post is opened containing the deprecated forms of the blocks, but I am guessing that is due to the deprecated forms not matching the other even older deprecated forms before matching the last one in the list. The content is preserved and automatically converted as expected and no blocks are made invalid. After saving, no errors appear in the JavaScript console.
(I would say that the failed matches of deprecated forms should not be called out in the JavaScript console as errors, but I am not sure of the history of this functionality or how it works, and that is out-of-scope for this PR anyway.)
Additional info